-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: TET-363 tag #73
Conversation
src/components/Tag/Tag.styles.ts
Outdated
label: BaseProps; | ||
closeButton?: BaseProps; | ||
}; | ||
beforeComponent: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
beforeComponent should be in innerElements, and optional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks, really good, minor comments
src/components/Tag/Tag.styles.ts
Outdated
label: BaseProps; | ||
closeButton?: BaseProps; | ||
}; | ||
beforeComponent: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should also be optional, so that when user is passing the custom prop it won't require it
state: (typeof states)[number]; | ||
}) => ( | ||
<tet.div flexBasis="120px" flexGrow="1" flexShrink="0"> | ||
<Tag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Tag in TagDocs should be clickable to feel more "interactive" so I would pass some empty "onClick" here
[onCloseClick], | ||
); | ||
|
||
const containerRef = useRef<HTMLSpanElement | null>(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move it before handleOnClose Click
return { | ||
container: { | ||
...container, | ||
...(hasOnClick ? hasOnClickStyles : {}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be also ...(hasOnClick && hasOnClickStyles)
src/components/Tag/Tag.tsx
Outdated
onClick={handleOnCloseClick} | ||
state={state} | ||
{...styles.closeButton} | ||
data-testid="tag-icon-button" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tag-iconButton
src/components/Tag/Tag.tsx
Outdated
size="2xSmall" | ||
{...beforeComponent.props} | ||
{...styles.avatar} | ||
data-testid="tag-before-component" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tag-avatar
src/components/Tag/Tag.tsx
Outdated
> | ||
{label} | ||
</tet.p> | ||
{!!onCloseClick && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{!!onCloseClick && ( | |
{hasCloseButton && ( |
PR moved to https://github.com/VirtusLab/tetrisly-react/pull/89/files on rebased (with main) branch. |
No description provided.